Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

pass <package> to dlv test/debug if applicable #1164

Merged
merged 2 commits into from
Sep 28, 2017
Merged

pass <package> to dlv test/debug if applicable #1164

merged 2 commits into from
Sep 28, 2017

Conversation

zhouhaibing089
Copy link
Contributor

@zhouhaibing089 zhouhaibing089 commented Aug 25, 2017 •
edited
Loading

we should prefer passing package name to dlv test or dlv debug. this fixes the issue mentioned at #846 (comment).

@ramya-rao-a
Copy link
Contributor

@zhouhaibing089 Your issue in #846 (comment) is about running tests where as your PR here is in the debugging area. How will it help in fixing the issue of running tests?

@zhouhaibing089
Copy link
Contributor Author

@ramya-rao-a the change in src/testUtils.ts is about the fix on running test.

ramya-rao-a
ramya-rao-a reviewed Sep 3, 2017
View reviewed changes
src/testUtils.ts
@@ -123,6 +123,11 @@ export function goTest(testconfig: TestConfig): Thenable<boolean> {
return Promise.resolve();
}

// append the package name to args if applicable
Copy link
Contributor

@ramya-rao-a ramya-rao-a Sep 3, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • this might not work in case of multiple GOPATHs, because then process.env['GOPATH'] can have : or ; separated paths. Follow https://github.com/Microsoft/vscode-go/blob/0.6.63/src/goCheck.ts#L180-L181 instead
  • we are moving away from using GOPATH from process.env. Instead use getCurrentGoPath()
  • When the command Go: Test All Packages In Workspace is run, this go test command will fail with "can't load package.. no buildable Go source files in ..." if the workspace itself is just a folder containing packages and not a package itself. So one way is to have the package import path as part of the TestConfig object

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PTAL again, I am not sure how the third item can be solved(and seems it is more than the effort here)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

testconfig.includeSubDirectories is true only for the "test all pks in workspace" scenario.
So you can add a check to ensure testconfig.includeSubDirectories is false before adding the importPath

@microsoft microsoft deleted a comment from msftclas Sep 12, 2017
@msftclas
Copy link

msftclas commented Sep 15, 2017 •
edited
Loading

CLA assistant check
All CLA requirements met.

@zhouhaibing089 zhouhaibing089 force-pushed the dlv-test-pkg branch 2 times, most recently from 1566dd8 to 904f70c Compare September 15, 2017 07:44
@ramya-rao-a ramya-rao-a merged commit 2520e29 into microsoft:master Sep 28, 2017
@ramya-rao-a
Copy link
Contributor

@zhouhaibing089 One issue with passing the package to go test is that the log.Println or fmt.Println from the tests are no longer getting piped to stdout

@rakyll @francesc Would you know why go test would print out the log.Println and fmt.Println statements but go test import-path-to-my-package doesn't?

@zhouhaibing089
Copy link
Contributor Author

go test -v import-path-to-my-package will do.

@zhouhaibing089
Copy link
Contributor Author

If this is a regression issue, I propose that we add a configuration like: test.verbose, if it is true, we add the flag -v, otherwise suppress the output.

@rakyll
Copy link

rakyll commented Oct 2, 2017

go test and go test should be behave similarly, not outputting to stdout if -v is not set. Can you file an issue if that's not the case?

@ramya-rao-a
Copy link
Contributor

Looks like its a platform issue

In Windows go test -v import-path-to-my-package prints the text from log.Println statements
But in Mac it doesn't. Both are using Go 1.9

Logged golang/go#22113

@ramya-rao-a
Copy link
Contributor

Nope not a platform issue.
It was all me and my duplicate set up of the same project in the default GOPATH and my other GOPATH.

I was making the changes in one and go test package-path was testing the other.

All is well with the world folks, sorry for the churn

@ramya-rao-a ramya-rao-a mentioned this pull request Oct 6, 2017
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@ramya-rao-a ramya-rao-a ramya-rao-a left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@zhouhaibing089 @ramya-rao-a @msftclas @rakyll @msftgits